monai/losses/cldice.py (2)

187-187: Add stacklevel=2 to warnings.

Without stacklevel, warnings point to this line instead of the caller's location.

Proposed fix

-                warnings.warn("single channel prediction, `softmax=True` ignored.")
+                warnings.warn("single channel prediction, `softmax=True` ignored.", stacklevel=2)

Same applies to lines 196 and 202.

---

340-340: Add stacklevel=2 here as well.

Proposed fix

-                warnings.warn("single channel prediction, `to_onehot_y=True` ignored.")
+                warnings.warn("single channel prediction, `to_onehot_y=True` ignored.", stacklevel=2)

tests/losses/test_cldice_loss.py (2)

36-71: Consider adding a test case for include_background=False.

Current cases cover sigmoid, softmax, and to_onehot_y, but not background exclusion.

---

82-87: Consider using @unittest.skipUnless decorator.

More idiomatic than early return.

Proposed fix

+    `@unittest.skipUnless`(torch.cuda.is_available(), "CUDA not available")
     def test_cuda(self):
-        if not torch.cuda.is_available():
-            return
         loss = SoftclDiceLoss()
         result = loss(ONES_2D["input"].cuda(), ONES_2D["target"].cuda())
         np.testing.assert_allclose(result.detach().cpu().numpy(), 0.0, atol=1e-4)

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

monai/losses/cldice.py (6)

188-189: Add stacklevel=2 to warnings.

Without it, warnings point to this file instead of the caller's location.

Proposed fix

             if n_pred_ch == 1:
-                warnings.warn("single channel prediction, `softmax=True` ignored.")
+                warnings.warn("single channel prediction, `softmax=True` ignored.", stacklevel=2)

Apply similarly to lines 198 and 204.

Also applies to: 197-198, 203-204

---

232-233: Inconsistent reduction="none" behavior with DiceLoss.

DiceLoss applies .view(-1) for reduction="none". Here it's left as-is. This may cause issues when combining losses or stacking results.

Proposed fix for consistency

         elif self.reduction == LossReduction.NONE.value:
-            pass  # keep per-batch values
+            cl_dice = cl_dice.view(-1)

---

293-315: Consider validating alpha range.

alpha outside [0, 1] would produce unusual weighting. While possibly intentional, a validation or warning could prevent mistakes.

Proposed fix

         if smooth <= 0:
             raise ValueError(f"smooth must be a positive value but got {smooth}.")
+        if not 0.0 <= alpha <= 1.0:
+            warnings.warn(f"alpha={alpha} is outside [0, 1], loss weighting may be unusual.", stacklevel=2)

---

344-344: Add stacklevel=2 here as well.

-                warnings.warn("single channel prediction, `to_onehot_y=True` ignored.")
+                warnings.warn("single channel prediction, `to_onehot_y=True` ignored.", stacklevel=2)

---

171-181: Missing Returns section in docstring.

Per coding guidelines, return values should be documented.

Proposed addition

         Raises:
             AssertionError: When input and target (after one hot transform if set)
                 have different shapes.
+
+        Returns:
+            torch.Tensor: The computed clDice loss, reduced according to `self.reduction`.

         """

---

318-328: Missing Returns section in docstring.

Same as SoftclDiceLoss.forward.

Proposed addition

         Raises:
             ValueError: When number of dimensions for input and target are different.
             ValueError: When number of channels for target is neither 1 nor the same as input.
+
+        Returns:
+            torch.Tensor: The weighted combination of Dice and clDice losses.

         """

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

monai/losses/cldice.py (3)

182-191: Add Returns section to the forward docstring.
Required by the project docstring guidelines.

📝 Example

         Raises:
             AssertionError: When input and target (after one hot transform if set)
                 have different shapes.

+        Returns:
+            Loss value. Scalar for "mean"/"sum", or per-batch tensor for "none".

---

304-306: Set base _Loss.reduction for consistency.
Right now the module reports the default reduction even if a different one is passed.

♻️ Suggested tweak

-        super().__init__()
+        super().__init__(reduction=LossReduction(reduction).value)

---

337-346: Add Returns section to the forward docstring.
Required by the project docstring guidelines.

📝 Example

         Raises:
             ValueError: When number of dimensions for input and target are different.
             ValueError: When number of channels for target is neither 1 nor the same as input.

+        Returns:
+            Loss value. Scalar for "mean"/"sum", or per-batch tensor for "none".

tests/losses/test_cldice_loss.py (2)

74-109: Add docstrings for new test definitions.
Project guidelines require Google-style docstrings for each definition.

---

111-135: Add a to_onehot_y=True combined-loss test.
This new option in SoftDiceclDiceLoss isn’t exercised yet.

tests/losses/test_cldice_loss.py (3)

101-107: No test exercises the deprecated y_pred/y_true argument mapping.

The @deprecated_arg wrappers on both forward() methods are untested — neither a warning assertion nor a value assertion confirms they work.

🧪 Suggested addition

def test_deprecated_args(self):
    import warnings
    loss = SoftclDiceLoss()
    inp = ONES_2D["input"]
    tgt = ONES_2D["target"]
    with warnings.catch_warnings(record=True) as w:
        warnings.simplefilter("always")
        result = loss.forward(y_pred=inp, y_true=tgt)
    self.assertTrue(any("y_pred" in str(warning.message) for warning in w))
    np.testing.assert_allclose(result.detach().cpu().numpy(), 0.0, atol=1e-4)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/losses/test_cldice_loss.py` around lines 101 - 107, Add a unit test
that exercises the deprecated y_pred/y_true argument mapping on
SoftclDiceLoss.forward: instantiate SoftclDiceLoss, call
loss.forward(y_pred=inp, y_true=tgt) inside a
warnings.catch_warnings(record=True) with warnings.simplefilter("always"),
assert a deprecation warning mentions "y_pred" or "y_true", and assert the
returned loss value equals the expected numeric result (e.g., 0.0 for ONES_2D
input) using numpy/testing or torch assertions; this verifies the
`@deprecated_arg` wrapper on SoftclDiceLoss.forward actually emits the warning and
returns the correct value.

---

81-86: Use self.skipTest() instead of bare return in CUDA guards.

return makes the test appear as passed in reports; self.skipTest() correctly marks it as skipped.

♻️ Proposed fix (same pattern for both classes)

     def test_cuda(self):
         if not torch.cuda.is_available():
-            return
+            self.skipTest("CUDA not available")

Also applies to: 117-122

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/losses/test_cldice_loss.py` around lines 81 - 86, The test_cuda methods
currently use a bare return when CUDA is unavailable; replace that with
self.skipTest("CUDA not available") so the test is marked skipped instead of
passed. Update the test method named test_cuda (the one that constructs
SoftclDiceLoss and calls loss(ONES_2D["input"].cuda(),
ONES_2D["target"].cuda())) and the analogous test_cuda in the other test class
(the second occurrence around the ONES_2D usage) to call self.skipTest("CUDA not
available") at the top of the method when torch.cuda.is_available() is False.

---

66-71: COMBINED_CASES has no to_onehot_y=True test for SoftDiceclDiceLoss.

The to_onehot_y path in SoftDiceclDiceLoss.forward() (Lines 353–358 of cldice.py) is entirely unexercised. Add at least one case, e.g., alpha=0.5, single-channel B1H[WD] target with multi-channel input.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/losses/test_cldice_loss.py` around lines 66 - 71, COMBINED_CASES
currently lacks a test exercising the to_onehot_y=True branch in
SoftDiceclDiceLoss.forward(); add a new case to COMBINED_CASES that uses
alpha=0.5 (or similar), sets to_onehot_y=True, and supplies a single-channel
target (e.g., shape B1HWD or B1HW) with a corresponding multi-channel prediction
so the loss converts target to one-hot; this will ensure the to_onehot_y path in
SoftDiceclDiceLoss.forward() is executed during tests.

monai/losses/cldice.py (2)

306-315: smooth_nr=1.0 / smooth_dr=1.0 passed to DiceLoss differ from DiceLoss's own defaults (1e-5).

Users swapping from standalone DiceLoss to SoftDiceclDiceLoss will see a different Dice component numerically. This is intentional (clDice convention), but the docstring is silent on it. Consider adding a note like: "Note: smooth_nr and smooth_dr default to 1.0 (clDice convention), which differs from standalone DiceLoss defaults of 1e-5."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@monai/losses/cldice.py` around lines 306 - 315, The Dice component of
SoftDiceclDiceLoss passes smooth_nr=1.0 and smooth_dr=1.0 into DiceLoss (see the
self.dice = DiceLoss(...) call), which differs from DiceLoss's defaults of 1e-5;
update the SoftDiceclDiceLoss class/function docstring to include a short note
stating that smooth_nr and smooth_dr default to 1.0 following the clDice
convention (and that this differs from standalone DiceLoss defaults of 1e-5) so
users are aware of the numerical difference when switching between losses.

---

190-218: SoftclDiceLoss.forward() skips dimension/channel pre-validation.

SoftDiceclDiceLoss.forward() raises a clear ValueError for dim/channel mismatches (Lines 343–351), but SoftclDiceLoss.forward() only catches shape mismatches after all preprocessing via an opaque AssertionError. Standalone callers passing a single-channel target against a multi-channel input with to_onehot_y=False get no actionable message.

♻️ Proposed fix

+        if input.dim() != target.dim():
+            raise ValueError(
+                f"input and target must have the same number of dimensions, "
+                f"got {input.shape} and {target.shape}."
+            )
+        if target.shape[1] != 1 and target.shape[1] != input.shape[1]:
+            raise ValueError(
+                f"number of target channels is neither 1 nor equal to input channels, "
+                f"got {input.shape} and {target.shape}."
+            )
         n_pred_ch = input.shape[1]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@monai/losses/cldice.py` around lines 190 - 218, SoftclDiceLoss.forward
currently only raises an AssertionError after preprocessing, making
channel/shape errors opaque; update SoftclDiceLoss.forward to perform explicit
channel/shape validation early (before applying sigmoid/softmax/other_act and
before to_onehot_y handling) and raise a clear ValueError like
SoftDiceclDiceLoss.forward does: check n_pred_ch = input.shape[1] versus target
channel dimension (or whether to_onehot_y is False and target has single
channel) and verify compatibility (e.g., target.shape == input.shape or target
channels == 1 when to_onehot_y=True), and replace the late
AssertionError(f"ground truth has different shape...") with a ValueError
containing an actionable message mentioning the function name and the mismatched
shapes.